Skip to content

Conversation

@JohnRichard4096
Copy link
Member

@JohnRichard4096 JohnRichard4096 commented Aug 4, 2025

close #79

Sourcery 总结

重构仓库层为独立模块,引入通过任意属性查询货币的功能,并扩展账户模型以包含冻结标志

新功能:

  • 添加 AccountRepository、CurrencyRepository 和 TransactionRepository 模块
  • 添加 API 和服务 get_currency_by_kwargs 以通过任意字段获取货币
  • UserAccountData Pydantic 模型添加 frozen 字段

改进:

  • 将单一的 repository.py 重构为独立的仓库类并重新导出它们
  • CurrencyRepository.get_currency 中使用 singledispatch 以支持 ID 和 kwargs 查找
  • 更新 API 和服务层以与新的仓库结构集成
Original summary in English

Summary by Sourcery

Restructure repository layer into separate modules, introduce querying currencies by arbitrary attributes, and extend account model with a frozen flag

New Features:

  • Add AccountRepository, CurrencyRepository, and TransactionRepository modules
  • Add API and service get_currency_by_kwargs to fetch currency by arbitrary fields
  • Add frozen field to UserAccountData Pydantic model

Enhancements:

  • Refactor monolithic repository.py into separate repository classes and re-export them
  • Use singledispatch in CurrencyRepository.get_currency to support ID and kwargs lookup
  • Update API and service layers to integrate with the new repository structure

Sourcery 总结

将数据访问层重构为独立的仓库模块,支持通过任意关键字参数查找货币,并扩展账户模型以支持冻结功能

新功能:

  • 添加 get_currency_by_kwargs 服务和 API,以通过任意属性查询货币
  • 在 UserAccountData 模型中引入 frozen 标志以支持账户冻结

改进:

  • 将单一的 repository.py 重构为独立的 AccountRepository、CurrencyRepository 和 TransactionRepository 模块
  • 在 CurrencyRepository 中使用 singledispatch 实现灵活的 get_currency 重载,并使服务/API 层与新的仓库结构保持一致
Original summary in English

Sourcery 总结

将仓库层重构为专用模块,支持通过任意属性灵活查询货币,并添加账户冻结功能

新功能:

  • 添加 get_currency_by_kwargs API 和服务,用于根据任意字段获取货币
  • 在 UserAccountData 模型中引入 frozen 标志以支持账户冻结

改进:

  • 将单一的 repository.py 拆分为独立的 AccountRepository、CurrencyRepository 和 TransactionRepository 模块
  • 在 CurrencyRepository 中使用 singledispatch 并添加 get_currency_by_kwargs,用于基于属性的查询
  • 更新 API 和服务层,以与新的仓库结构集成

构建:

  • 将插件版本从 0.1.5 提升到 0.1.6
Original summary in English

Sourcery 摘要

重构数据访问层为独立的仓库模块,支持按任意属性查询货币,并扩展账户模型,增加冻结标志

新功能:

  • 添加专用的 AccountRepositoryCurrencyRepositoryTransactionRepository 模块
  • 引入 get_currency_by_kwargs API 和服务函数,用于按任意字段查询货币
  • UserAccountData 模型添加 frozen 标志,以启用账户冻结

改进:

  • 将单一的 repository.py 重构为独立的仓库类并重新导出它们
  • CurrencyRepository.get_currency 中使用 singledispatch 以实现灵活查找
  • 更新 API 和服务层,以与新的仓库结构集成

构建:

  • 将项目版本提升到 0.1.6
Original summary in English

Summary by Sourcery

Refactor the data access layer into dedicated repository modules, support querying currencies by arbitrary attributes, and extend the account model with a frozen flag

New Features:

  • Add dedicated AccountRepository, CurrencyRepository, and TransactionRepository modules
  • Introduce get_currency_by_kwargs API and service function for querying currencies by arbitrary fields
  • Add frozen flag to UserAccountData model to enable account freezing

Enhancements:

  • Refactor monolithic repository.py into separate repository classes and re-export them
  • Employ singledispatch in CurrencyRepository.get_currency for flexible lookup
  • Update API and service layers to integrate with the new repository structure

Build:

  • Bump project version to 0.1.6

@JohnRichard4096
Copy link
Member Author

@sourcery-ai review

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Aug 4, 2025

审阅者指南

此 PR 重构了数据访问层,将其拆分为独立的仓库模块,增强了货币查找功能以接受任意查询参数,并将账户冻结标志集成到账户模型和仓库中。

UserAccountData 冻结字段添加的 ER 图

erDiagram
    USER_ACCOUNT_DATA {
        id string
        currency_id string
        balance float
        last_updated datetime
        frozen bool
    }
Loading

重构后的仓库结构的类图

classDiagram
    class AccountRepository {
        +__init__(session)
        +get_or_create_account(user_id, currency_id)
        +set_account_frozen(account_id, currency_id, frozen)
        +set_frozen_all(account_id, frozen)
        +is_account_frozen(account_id, currency_id)
        +get_balance(account_id, currency_id)
        +update_balance(account_id, amount, currency_id)
        +list_accounts(currency_id)
        +remove_account(account_id, currency_id)
    }
    class CurrencyRepository {
        +__init__(session)
        +get_currency(currency_id)
        +get_currency_by_kwargs(**kwargs)
        +get_or_create_currency(currency_data)
        +createcurrency(currency_data)
        +update_currency(currency_data)
        +list_currencies()
        +remove_currency(currency_id)
    }
    class TransactionRepository {
        +__init__(session)
        +create_transaction(account_id, currency_id, amount, action, source, balance_before, balance_after, timestamp)
        +get_transaction_history(account_id, limit)
        +get_transaction_history_by_time_range(account_id, start_time, end_time, limit)
        +remove_transaction(transaction_id)
    }
Loading

带有冻结标志的 UserAccountData 类图

classDiagram
    class UserAccountData {
        +id: str
        +currency_id: str
        +balance: float
        +last_updated: datetime
        +frozen: bool
    }
Loading

CurrencyRepository 货币查找方法的类图

classDiagram
    class CurrencyRepository {
        +get_currency(currency_id)
        +get_currency_by_kwargs(**kwargs)
    }
Loading

文件级别更改

更改 详情 文件
仓库层模块化
  • 将单一的 repository.py 拆分为账户、货币和交易仓库模块
  • 更新了根 repository.py 以导入并重新导出新的仓库类
nonebot_plugin_value/repository.py
nonebot_plugin_value/repositories/account.py
nonebot_plugin_value/repositories/currency.py
nonebot_plugin_value/repositories/transaction.py
支持通过任意字段查找货币
  • CurrencyRepository 中添加了 get_currency_by_kwargs 方法
  • singledispatch 应用于 get_currency 以实现基于类型的分派
  • 在服务和 API 层引入了 get_currency_by_kwargs
nonebot_plugin_value/repositories/currency.py
nonebot_plugin_value/services/currency.py
nonebot_plugin_value/api/api_currency.py
添加和管理账户冻结标志
  • UserAccountData Pydantic 模型添加了 frozen 字段
  • AccountRepository 中实现了 set_account_frozenset_frozen_allis_account_frozen
nonebot_plugin_value/pyd_models/balance_pyd.py
nonebot_plugin_value/repositories/account.py

对关联问题的评估

问题 目标 已解决 解释
#79 添加通过name查找Currency的功能
#79 更新Account的PydanticModel,因为它缺少了frozen字段

可能关联的问题


提示和命令

与 Sourcery 互动

  • 触发新审查: 在拉取请求上评论 @sourcery-ai review
  • 继续讨论: 直接回复 Sourcery 的审查评论。
  • 从审查评论生成 GitHub 问题: 通过回复 Sourcery 的审查评论,要求它创建一个问题。您也可以回复审查评论并加上 @sourcery-ai issue 来创建问题。
  • 生成拉取请求标题: 随时在拉取请求标题中任意位置写入 @sourcery-ai,即可生成标题。您也可以随时在拉取请求上评论 @sourcery-ai title 来(重新)生成标题。
  • 生成拉取请求摘要: 随时在拉取请求正文中任意位置写入 @sourcery-ai summary,即可在您想要的位置生成 PR 摘要。您也可以随时在拉取请求上评论 @sourcery-ai summary 来(重新)生成摘要。
  • 生成审阅者指南: 随时在拉取请求上评论 @sourcery-ai guide 来(重新)生成审阅者指南。
  • 解决所有 Sourcery 评论: 在拉取请求上评论 @sourcery-ai resolve 以解决所有 Sourcery 评论。如果您已经处理了所有评论并且不想再看到它们,这会很有用。
  • 驳回所有 Sourcery 审查: 在拉取请求上评论 @sourcery-ai dismiss 以驳回所有现有 Sourcery 审查。如果您想重新开始新的审查,这特别有用——别忘了评论 @sourcery-ai review 以触发新的审查!

自定义您的体验

访问您的 仪表板 以:

  • 启用或禁用审查功能,例如 Sourcery 生成的拉取请求摘要、审阅者指南等。
  • 更改审查语言。
  • 添加、删除或编辑自定义审查说明。
  • 调整其他审查设置。

获取帮助

Original review guide in English

Reviewer's Guide

This PR restructures the data-access layer into discrete repository modules, enhances currency lookup to accept arbitrary query parameters, and integrates an account freeze flag into the account model and repository.

ER diagram for UserAccountData frozen field addition

erDiagram
    USER_ACCOUNT_DATA {
        id string
        currency_id string
        balance float
        last_updated datetime
        frozen bool
    }
Loading

Class diagram for refactored repository structure

classDiagram
    class AccountRepository {
        +__init__(session)
        +get_or_create_account(user_id, currency_id)
        +set_account_frozen(account_id, currency_id, frozen)
        +set_frozen_all(account_id, frozen)
        +is_account_frozen(account_id, currency_id)
        +get_balance(account_id, currency_id)
        +update_balance(account_id, amount, currency_id)
        +list_accounts(currency_id)
        +remove_account(account_id, currency_id)
    }
    class CurrencyRepository {
        +__init__(session)
        +get_currency(currency_id)
        +get_currency_by_kwargs(**kwargs)
        +get_or_create_currency(currency_data)
        +createcurrency(currency_data)
        +update_currency(currency_data)
        +list_currencies()
        +remove_currency(currency_id)
    }
    class TransactionRepository {
        +__init__(session)
        +create_transaction(account_id, currency_id, amount, action, source, balance_before, balance_after, timestamp)
        +get_transaction_history(account_id, limit)
        +get_transaction_history_by_time_range(account_id, start_time, end_time, limit)
        +remove_transaction(transaction_id)
    }
Loading

Class diagram for UserAccountData with frozen flag

classDiagram
    class UserAccountData {
        +id: str
        +currency_id: str
        +balance: float
        +last_updated: datetime
        +frozen: bool
    }
Loading

Class diagram for CurrencyRepository currency lookup methods

classDiagram
    class CurrencyRepository {
        +get_currency(currency_id)
        +get_currency_by_kwargs(**kwargs)
    }
Loading

File-Level Changes

Change Details Files
Modularize repository layer
  • Split monolithic repository.py into Account, Currency, and Transaction repository modules
  • Updated root repository.py to import and re-export the new repository classes
nonebot_plugin_value/repository.py
nonebot_plugin_value/repositories/account.py
nonebot_plugin_value/repositories/currency.py
nonebot_plugin_value/repositories/transaction.py
Support currency lookup by arbitrary fields
  • Added get_currency_by_kwargs method in CurrencyRepository
  • Applied singledispatch to get_currency for type-based dispatch
  • Introduced get_currency_by_kwargs in service and API layers
nonebot_plugin_value/repositories/currency.py
nonebot_plugin_value/services/currency.py
nonebot_plugin_value/api/api_currency.py
Add and manage account freeze flag
  • Added frozen field to UserAccountData Pydantic model
  • Implemented set_account_frozen, set_frozen_all, and is_account_frozen in AccountRepository
nonebot_plugin_value/pyd_models/balance_pyd.py
nonebot_plugin_value/repositories/account.py

Assessment against linked issues

Issue Objective Addressed Explanation
#79 添加通过name查找Currency的功能
#79 更新Account的PydanticModel,因为它缺少了frozen字段

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

sourcery-ai[bot]

This comment was marked as outdated.

@sourcery-ai sourcery-ai bot changed the title 添加frozen,更新API Refactor repository, add currency query by kwargs, account freeze Aug 4, 2025
@JohnRichard4096
Copy link
Member Author

JohnRichard4096 commented Aug 4, 2025

@sourcery-ai review

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohnRichard4096 - 我已审阅你的更改 - 以下是一些反馈:

  • CurrencyRepository.get_currency 上的 @singledispatch 没有注册备用处理程序——要么为其他输入类型添加注册,要么删除装饰器以避免混淆。
  • get_currency_by_kwargs 默默地忽略不支持的 kwargs,并且可能在空过滤器下运行——考虑验证输入键或在未提供有效过滤器时引发错误。
  • 在 api_currency.py 中,get_currency_by_kwargs 调用了不存在的 __currency_by_kwargs——更新 API 以调用正确的服务函数名称。
AI 代理提示
请解决此代码审查中的评论:
## 总体评论
- CurrencyRepository.get_currency 上的 @singledispatch 没有注册备用处理程序——要么为其他输入类型添加注册,要么删除装饰器以避免混淆。
- get_currency_by_kwargs 默默地忽略不支持的 kwargs,并且可能在空过滤器下运行——考虑验证输入键或在未提供有效过滤器时引发错误。
- 在 api_currency.py 中,get_currency_by_kwargs 调用了不存在的 __currency_by_kwargs——更新 API 以调用正确的服务函数名称。

## 单独评论

### 评论 1
<location> `nonebot_plugin_value/repositories/currency.py:69` </location>
<code_context>
+            result = await self.createcurrency(currency_data)
+            return result, False
+
+    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
+        async with self.session as session:
+            """创建新货币"""
</code_context>

<issue_to_address>
方法名“createcurrency”不符合标准 Python 命名约定。

重命名为“create_currency”以符合 PEP8 规范并提高可读性。
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
            result = await self.createcurrency(currency_data)
            return result, False

    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
        async with self.session as session:
            """创建新货币"""
=======
            result = await self.create_currency(currency_data)
            return result, False

    async def create_currency(self, currency_data: CurrencyData) -> CurrencyMeta:
        async with self.session as session:
            """创建新货币"""
>>>>>>> REPLACE

</suggested_fix>

### 评论 2
<location> `nonebot_plugin_value/repositories/account.py:115` </location>
<code_context>
+    async def update_balance(
</code_context>

<issue_to_address>
The update_balance 方法将余额设置为“amount”而不是增加/减少。

目前,account.balance = amount 会替换现有余额。请使用 account.balance += amount 来递增调整余额。如果目的是覆盖,请澄清参数名称。
</issue_to_address>

### 评论 3
<location> `nonebot_plugin_value/repositories/transaction.py:47` </location>
<code_context>
+                balance_after=balance_after,
+                timestamp=timestamp,
+            )
+            session.add(transaction_data)
+            await session.commit()
+            await session.refresh(transaction_data)
</code_context>

<issue_to_address>
commit 和 refresh 后重复的 session.add。

考虑删除 commit 和 refresh 后重复的 session.add(transaction_data) 以提高代码清晰度。
</issue_to_address>

Sourcery 对开源免费 - 如果你喜欢我们的评论,请考虑分享 ✨
帮助我更有用!请点击每个评论上的 👍 或 👎,我会根据反馈改进你的评论。
Original comment in English

Hey @JohnRichard4096 - I've reviewed your changes - here's some feedback:

  • The @singledispatch on CurrencyRepository.get_currency has no registered alternate handlers—either add registrations for other input types or remove the decorator to avoid confusion.
  • get_currency_by_kwargs silently ignores unsupported kwargs and may run with an empty filter—consider validating input keys or raising an error when no valid filters are provided.
  • In api_currency.py, get_currency_by_kwargs calls __currency_by_kwargs which doesn’t exist—update the API to call the correct service function name.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The @singledispatch on CurrencyRepository.get_currency has no registered alternate handlers—either add registrations for other input types or remove the decorator to avoid confusion.
- get_currency_by_kwargs silently ignores unsupported kwargs and may run with an empty filter—consider validating input keys or raising an error when no valid filters are provided.
- In api_currency.py, get_currency_by_kwargs calls __currency_by_kwargs which doesn’t exist—update the API to call the correct service function name.

## Individual Comments

### Comment 1
<location> `nonebot_plugin_value/repositories/currency.py:69` </location>
<code_context>
+            result = await self.createcurrency(currency_data)
+            return result, False
+
+    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
+        async with self.session as session:
+            """创建新货币"""
</code_context>

<issue_to_address>
Method name 'createcurrency' does not follow standard Python naming conventions.

Rename to 'create_currency' for PEP8 compliance and better readability.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
            result = await self.createcurrency(currency_data)
            return result, False

    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
        async with self.session as session:
            """创建新货币"""
=======
            result = await self.create_currency(currency_data)
            return result, False

    async def create_currency(self, currency_data: CurrencyData) -> CurrencyMeta:
        async with self.session as session:
            """创建新货币"""
>>>>>>> REPLACE

</suggested_fix>

### Comment 2
<location> `nonebot_plugin_value/repositories/account.py:115` </location>
<code_context>
+    async def update_balance(
</code_context>

<issue_to_address>
The update_balance method sets the balance to 'amount' instead of incrementing/decrementing.

Currently, account.balance = amount replaces the existing balance. Use account.balance += amount to adjust the balance incrementally. If overwriting is intended, clarify the parameter name.
</issue_to_address>

### Comment 3
<location> `nonebot_plugin_value/repositories/transaction.py:47` </location>
<code_context>
+                balance_after=balance_after,
+                timestamp=timestamp,
+            )
+            session.add(transaction_data)
+            await session.commit()
+            await session.refresh(transaction_data)
</code_context>

<issue_to_address>
Redundant session.add after commit and refresh.

Consider removing the redundant session.add(transaction_data) after commit and refresh to improve code clarity.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

JohnRichard4096 and others added 5 commits August 4, 2025 12:19
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@JohnRichard4096 JohnRichard4096 merged commit b5fd8d0 into master Aug 4, 2025
1 check passed
@JohnRichard4096 JohnRichard4096 deleted the feat-upd branch August 4, 2025 04:23
Copy link

@SourceryAI SourceryAI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohnRichard4096 - 我已经审阅了你的更改 - 以下是一些反馈:

  • CurrencyRepository.get_currency 上的 singledispatch 装饰器在实例方法上无法按预期工作(它将始终分派到存储库实例),因此请考虑切换到 functools.singledispatchmethod 或其他重载机制来区分 ID 与 kwargs 查找。
  • 在服务层 get_currency_by_kwargs 中,你将外部提供的 AsyncSession 封装在 async with 中,这可能会过早地关闭或提交会话——请直接使用传入的会话,而不是进入新的上下文。
  • get_currency_by_kwargs 当前会静默忽略未知过滤器键;为防止出现细微错误,你可能需要验证每个 kwarg 是否与 CurrencyMeta 属性匹配,或者对无效字段引发错误。
AI 代理的提示
请解决此代码审查中的注释:
## 总体注释
- CurrencyRepository.get_currency 上的 singledispatch 装饰器在实例方法上无法按预期工作(它将始终分派到存储库实例),因此请考虑切换到 functools.singledispatchmethod 或其他重载机制来区分 ID 与 kwargs 查找。
- 在服务层 get_currency_by_kwargs 中,你将外部提供的 AsyncSession 封装在 async with 中,这可能会过早地关闭或提交会话——请直接使用传入的会话,而不是进入新的上下文。
- get_currency_by_kwargs 当前会静默忽略未知过滤器键;为防止出现细微错误,你可能需要验证每个 kwarg 是否与 CurrencyMeta 属性匹配,或者对无效字段引发错误。

## 独立注释

### 注释 1
<location> `nonebot_plugin_value/repositories/currency.py:67` </location>
<code_context>
+            result = await self.createcurrency(currency_data)
+            return result, False
+
+    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
+        async with self.session as session:
+            """创建新货币"""
</code_context>

<issue_to_address>
方法名 'createcurrency' 不符合标准 Python 命名约定。

重命名为 'create_currency' 以符合 PEP8 并保持一致性。
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
            result = await self.createcurrency(currency_data)
            return result, False

    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
        async with self.session as session:
            """创建新货币"""
            currency = CurrencyMeta(**currency_data.model_dump())
            session.add(currency)
            await session.commit()
            await session.refresh(currency)
            return currency
=======
            result = await self.create_currency(currency_data)
            return result, False

    async def create_currency(self, currency_data: CurrencyData) -> CurrencyMeta:
        async with self.session as session:
            """创建新货币"""
            currency = CurrencyMeta(**currency_data.model_dump())
            session.add(currency)
            await session.commit()
            await session.refresh(currency)
            return currency
>>>>>>> REPLACE

</suggested_fix>

@JohnRichard4096! 👋

感谢您通过 @sourcery-ai review 评论来试用 Sourcery!🚀

安装 sourcery-ai bot 以在每个拉取请求上获取自动代码审查 ✨

帮助我更有用!请点击每个评论上的 👍 或 👎,我将使用反馈来改进您的审查。
Original comment in English

Hey @JohnRichard4096 - I've reviewed your changes - here's some feedback:

  • The singledispatch decorator on CurrencyRepository.get_currency won’t work as intended on instance methods (it will always dispatch on the repository instance), so consider switching to functools.singledispatchmethod or another overload mechanism to distinguish ID vs kwargs lookups.
  • In the service layer get_currency_by_kwargs you wrap an externally provided AsyncSession in async with, which could close or commit the session prematurely—use the passed-in session directly instead of entering a new context.
  • get_currency_by_kwargs currently ignores unknown filter keys silently; to prevent subtle bugs you might validate that each kwarg matches a CurrencyMeta attribute or raise an error for invalid fields.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The singledispatch decorator on CurrencyRepository.get_currency won’t work as intended on instance methods (it will always dispatch on the repository instance), so consider switching to functools.singledispatchmethod or another overload mechanism to distinguish ID vs kwargs lookups.
- In the service layer get_currency_by_kwargs you wrap an externally provided AsyncSession in async with, which could close or commit the session prematurely—use the passed-in session directly instead of entering a new context.
- get_currency_by_kwargs currently ignores unknown filter keys silently; to prevent subtle bugs you might validate that each kwarg matches a CurrencyMeta attribute or raise an error for invalid fields.

## Individual Comments

### Comment 1
<location> `nonebot_plugin_value/repositories/currency.py:67` </location>
<code_context>
+            result = await self.createcurrency(currency_data)
+            return result, False
+
+    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
+        async with self.session as session:
+            """创建新货币"""
</code_context>

<issue_to_address>
Method name 'createcurrency' does not follow standard Python naming conventions.

Rename to 'create_currency' to align with PEP8 and maintain consistency.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
            result = await self.createcurrency(currency_data)
            return result, False

    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
        async with self.session as session:
            """创建新货币"""
            currency = CurrencyMeta(**currency_data.model_dump())
            session.add(currency)
            await session.commit()
            await session.refresh(currency)
            return currency
=======
            result = await self.create_currency(currency_data)
            return result, False

    async def create_currency(self, currency_data: CurrencyData) -> CurrencyMeta:
        async with self.session as session:
            """创建新货币"""
            currency = CurrencyMeta(**currency_data.model_dump())
            session.add(currency)
            await session.commit()
            await session.refresh(currency)
            return currency
>>>>>>> REPLACE

</suggested_fix>

Hi @JohnRichard4096! 👋

Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀

Install the sourcery-ai bot to get automatic code reviews on every pull request ✨

Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +64 to +74
result = await self.createcurrency(currency_data)
return result, False

async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
async with self.session as session:
"""创建新货币"""
currency = CurrencyMeta(**currency_data.model_dump())
session.add(currency)
await session.commit()
await session.refresh(currency)
return currency

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: 方法名 'createcurrency' 不符合标准 Python 命名约定。

重命名为 'create_currency' 以符合 PEP8 并保持一致性。

Suggested change
result = await self.createcurrency(currency_data)
return result, False
async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
async with self.session as session:
"""创建新货币"""
currency = CurrencyMeta(**currency_data.model_dump())
session.add(currency)
await session.commit()
await session.refresh(currency)
return currency
result = await self.create_currency(currency_data)
return result, False
async def create_currency(self, currency_data: CurrencyData) -> CurrencyMeta:
async with self.session as session:
"""创建新货币"""
currency = CurrencyMeta(**currency_data.model_dump())
session.add(currency)
await session.commit()
await session.refresh(currency)
return currency
Original comment in English

suggestion: Method name 'createcurrency' does not follow standard Python naming conventions.

Rename to 'create_currency' to align with PEP8 and maintain consistency.

Suggested change
result = await self.createcurrency(currency_data)
return result, False
async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
async with self.session as session:
"""创建新货币"""
currency = CurrencyMeta(**currency_data.model_dump())
session.add(currency)
await session.commit()
await session.refresh(currency)
return currency
result = await self.create_currency(currency_data)
return result, False
async def create_currency(self, currency_data: CurrencyData) -> CurrencyMeta:
async with self.session as session:
"""创建新货币"""
currency = CurrencyMeta(**currency_data.model_dump())
session.add(currency)
await session.commit()
await session.refresh(currency)
return currency

Copy link

@SourceryAI SourceryAI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohnRichard4096 - 我已审阅您的更改 - 以下是一些反馈:

  • API 函数 get_currency_by_kwargs 调用了一个未定义的函数 __currency_by_kwargs;请更新它以调用正确的服务或存储库方法。
  • CurrencyRepository.get_currency 使用 singledispatch 装饰,但没有注册的重载——要么删除 singledispatch,要么添加预期的特定类型实现。
  • 在 get_currency_by_kwargs 中,您默默地忽略了无效的属性键并允许空查询;请添加验证,以便缺失或不支持的 kwargs 会抛出错误并防止意外的全表扫描。
AI 代理的提示
请解决此代码审查中的评论:
## 总体评论
- API 函数 get_currency_by_kwargs 调用了一个未定义的函数 __currency_by_kwargs;请更新它以调用正确的服务或存储库方法。
- CurrencyRepository.get_currency 使用 singledispatch 装饰,但没有注册的重载——要么删除 singledispatch,要么添加预期的特定类型实现。
- 在 get_currency_by_kwargs 中,您默默地忽略了无效的属性键并允许空查询;请添加验证,以便缺失或不支持的 kwargs 会抛出错误并防止意外的全表扫描。

## 单独评论

### 评论 1
<location> `nonebot_plugin_value/repositories/currency.py:67` </location>
<code_context>
+            result = await self.createcurrency(currency_data)
+            return result, False
+
+    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
+        async with self.session as session:
+            """创建新货币"""
</code_context>

<issue_to_address>
方法名 'createcurrency' 与 Python 命名约定不一致。

将 'createcurrency' 重命名为 'create_currency',以符合 Python 标准和代码库的约定。
</issue_to_address>

@JohnRichard4096! 👋

感谢您通过评论 @sourcery-ai review 试用 Sourcery!🚀

安装 sourcery-ai bot 以在每个拉取请求上获得自动代码审查 ✨

帮助我更有用!请点击 👍 或 👎 对每个评论进行反馈,我将利用这些反馈来改进您的审查。
Original comment in English

Hey @JohnRichard4096 - I've reviewed your changes - here's some feedback:

  • The API function get_currency_by_kwargs calls an undefined function __currency_by_kwargs; update it to call the correct service or repository method.
  • CurrencyRepository.get_currency is decorated with singledispatch but has no registered overloads—either remove singledispatch or add the intended type‐specific implementations.
  • In get_currency_by_kwargs you silently ignore invalid attribute keys and allow empty queries; add validation so that missing or unsupported kwargs throw an error and prevent unintended full-table scans.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The API function get_currency_by_kwargs calls an undefined function __currency_by_kwargs; update it to call the correct service or repository method.
- CurrencyRepository.get_currency is decorated with singledispatch but has no registered overloads—either remove singledispatch or add the intended type‐specific implementations.
- In get_currency_by_kwargs you silently ignore invalid attribute keys and allow empty queries; add validation so that missing or unsupported kwargs throw an error and prevent unintended full-table scans.

## Individual Comments

### Comment 1
<location> `nonebot_plugin_value/repositories/currency.py:67` </location>
<code_context>
+            result = await self.createcurrency(currency_data)
+            return result, False
+
+    async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:
+        async with self.session as session:
+            """创建新货币"""
</code_context>

<issue_to_address>
Method name 'createcurrency' is inconsistent with Python naming conventions.

Rename 'createcurrency' to 'create_currency' for consistency with Python standards and the codebase.
</issue_to_address>

Hi @JohnRichard4096! 👋

Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀

Install the sourcery-ai bot to get automatic code reviews on every pull request ✨

Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

result = await self.createcurrency(currency_data)
return result, False

async def createcurrency(self, currency_data: CurrencyData) -> CurrencyMeta:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: 方法名“createcurrency”与 Python 命名约定不一致。

将“createcurrency”重命名为“create_currency”,以符合 Python 标准和代码库的约定。

Original comment in English

nitpick: Method name 'createcurrency' is inconsistent with Python naming conventions.

Rename 'createcurrency' to 'create_currency' for consistency with Python standards and the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[New Feature] 添加新的API

3 participants